-
Notifications
You must be signed in to change notification settings - Fork 0
Kh agent/vibe dev/pwa instructions #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8f2778e to
366487a
Compare
366487a to
0d59dd2
Compare
0d59dd2 to
4adef4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Progressive Web App (PWA) installation functionality, allowing users to install IONOS GPT as an app on their devices. The implementation includes platform-specific detection, installation prompts, and user interface components.
- Adds PWA detection and installation utilities with platform-specific logic for iOS/Safari vs other browsers
- Implements PWA install dialog with step-by-step instructions for iOS users
- Integrates PWA notifications into the existing notification system with appropriate event handling
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/i18n/locales/en-US/ionos.json | Adds English translations for PWA installation UI text |
| src/lib/i18n/locales/de-DE/ionos.json | Adds German translations for PWA installation UI text |
| src/lib/IONOS/services/pwa.ts | Core PWA service with platform detection and installation logic |
| src/lib/IONOS/components/notifications/PWAInstallDialog.svelte | Dialog component for PWA installation with iOS-specific instructions |
| src/lib/IONOS/components/notifications/NotificationManager.svelte | Integrates PWA notifications into notification system |
| src/lib/IONOS/components/notifications/NotificationBanner.svelte | Adds PWA install handling to notification banner |
| src/lib/IONOS/components/icons/Touch.svelte | Touch icon for PWA UI |
| src/lib/IONOS/components/icons/Share.svelte | Share icon for iOS installation instructions |
| src/lib/IONOS/components/common/Dialog.svelte | Adds external close button functionality for PWA dialog |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
27f58bc to
4e32818
Compare
4adef4e to
ffc7117
Compare
thlehmann-ionos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss.
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
src/lib/IONOS/components/notifications/NotificationManager.svelte
Outdated
Show resolved
Hide resolved
src/lib/IONOS/services/pwa.ts
Outdated
| /** | ||
| * Checks if we should show the PWA install prompt | ||
| */ | ||
| export function shouldShowPWAPrompt(deferredPrompt: BeforeInstallPromptEvent | null = null): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that it can be called with an optional event leads to confusion in some places where this function is called as at the calling point it does not work as expected.
|
|
||
| export interface BeforeInstallPromptEvent extends Event { | ||
| readonly platforms: string[]; | ||
| readonly userChoice: Promise<{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be its own type. In one place this property was cast to any. Its type could and should be declared in that place (which also gives a linter warning on any).
4ad0c63 to
d6012e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| else { | ||
| } |
Copilot
AI
Oct 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty else block should be removed as it serves no purpose and reduces code clarity.
| else { | |
| } |
c59b275 to
24be6ba
Compare
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
* Added external close-X below the content element * Removed overflow-hidden to allow new close-X to leave the container Notes: * Works for the privacy links banner * It should have a prop to enable this explicitly * Maybe it's better to have a special dialog fo such use cases * Maybe it should hide the inner button on mobile breakpoint and show the other one instead or maybe the inner one can be repurposed just by conditions Refs: PRODAI-380
To allow reuse in another place. Refs: PRODAI-380
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
Signed-off-by: Kai Henseler <[email protected]>
24be6ba to
de52e0f
Compare
No description provided.